Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change timing of requiring a loader for config to fix the issue #90 #93

Closed
wants to merge 1 commit into from
Closed

Change timing of requiring a loader for config to fix the issue #90 #93

wants to merge 1 commit into from

Conversation

sttk
Copy link
Contributor

@sttk sttk commented Apr 4, 2018

Preloading modules specified by opts.require and requiring a loader for config should move after respawning and ready to run a callback of launch to fix the issue #90.

@sttk sttk mentioned this pull request Apr 5, 2018
@sttk
Copy link
Contributor Author

sttk commented Feb 24, 2019

@phated Could you review this pr, too?

To fix the issue: gulp-cli #181, I was going to modify to export registerLoader of liftoff and use it like this.

However I've noticed that this pr can achieve to fix gulp-cli #181 with the modification of gulp-cli like:

(gulp-cli/index.js)
cli.prepareConfig = function(env, opts) {
  var cfgLoadOrder = ['home', 'cwd'];
  var cfg = loadConfigFiles(env.configFiles['.gulp'], cfgLoadOrder);
  opts = mergeConfigToCliFlags(opts, cfg);
  env = mergeConfigToEnvFlags(env, cfg);

  Liftoff.prototype.prepareConfig(env, opts);

};

@sttk sttk requested a review from phated February 24, 2019 10:41
@phated
Copy link
Member

phated commented Feb 25, 2019

I've rebased and made a couple of changes to this PR.

Closed by 6e280ae

Thanks for all the work on this stuff @sttk and I apologize for delaying review for so long!

@phated
Copy link
Member

phated commented Feb 25, 2019

@sttk Do you think the 2 changes you made are fine to be released as version 2.5.1 or should I do 2.6.0?

@phated phated closed this Feb 25, 2019
@sttk sttk deleted the move-preload-config branch February 26, 2019 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants